Skip to content

Correctly handle the "zh-Hans" language (bsc#1238584)#2314

Merged
lslezak merged 22 commits intomasterfrom
locale_fixes
May 7, 2025
Merged

Correctly handle the "zh-Hans" language (bsc#1238584)#2314
lslezak merged 22 commits intomasterfrom
locale_fixes

Conversation

@lslezak
Copy link
Copy Markdown
Contributor

@lslezak lslezak commented Apr 30, 2025 •
edited
Loading

Problem

  • The storage page crashes after selecting Chinese language ("zh-Hans")
  • https://bugzilla.suse.com/show_bug.cgi?id=1238584
  • A similar problem exists also with the pt_BR language as it also uses a country code. (All other languages use just a simple language code without any separator.) But it is less serious as the pt_BR locale is known by glibc so we do not need to do any additional conversions besides using the correct language separator.
  • The underlying problem is that Linux and the Web (browsers) use two different standards for identifying locales and languages: POSIX locale identifiers such as zh_TW.UTF-8 and IETF language tags such as zh-Hant. Agama sometimes uses a punctuation-only conversion which only works for some cases.

Solution

  • Change the zh_Hans in Weblate to zh_CN so we do not need to do any conversions
  • Change the web frontend to consistently use the dash (-) separator in all places, avoid unnecessary conversions, convert to underscore only when sending to the backend
  • Convert the underscore (_) separator to dash (-) when importing the frontend translations from Weblate
  • Handle exceptions in the Intl.ListFormat formatting function, in case of exception fallback to a simple formatting function with comma separator (", ")

Notes

  • It turned out that actually only the C.UTF-8 and en_US.UTF-8 locales were present in the Live ISO. Unfortunately the filtering regexp was not updated after renaming the PO translation files from po.<lang>.js to po-<lang>.js. 😱
  • I have changed the code to read the supported languages from the languages.json file. That allows more precise filtering. In the past we kept e.g. all de_DE, de_AT, de_CH for German language. Now we know that after selecting German the de_DE locale will be used so we can delete the de_AT and de_CH locales from the live ISO.

Testing

  • Added a new unit test
  • Tested manually
    • The web frontend does not crash a properly displays the formatted text, the Intl.ListFormat function is used in the New partitions will be created for "/" <and> "swap" text
    • The backend receives a valid Linux locale (zh_CN.UTF-8)
    • The Web UI translation files use the dash separator
    • The Live ISO contains all supported locales (locale -a)

agama-zh-CN

image

mvidner
mvidner requested changes Apr 30, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@mvidner mvidner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Some more cleanup in the code you have already touched
  2. Unfortunately Weblate is giving us a hybrid of the two language standards, IMHO we should fight back or be forever stuck with a mess

Comment thread web/src/context/installerL10n.tsx Outdated
Comment thread web/src/context/installerL10n.tsx
@@ -127,7 +127,13 @@ function languageFromLocale(locale: string): string {
* @see https://www.rfc-editor.org/info/bcp78
*/
function languageToLocale(language: string): string {
Copy link
Copy Markdown
Contributor

@mvidner mvidner Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please note there is also an inverse languageFromLocale function above, which also needs fixing, and testing

Comment thread web/src/agama.ts Outdated
Comment thread web/src/agama.test.ts Outdated
Comment thread live/src/config.sh Outdated
Comment thread web/package/agama-web-ui.changes Outdated
@imobachgs
Copy link
Copy Markdown
Contributor

imobachgs commented May 2, 2025

May I suggest to prevent crashes by catching exceptions in the formatList function?

dgdavid
dgdavid reviewed May 6, 2025
View reviewed changes
Comment thread web/src/agama.ts Outdated
imobachgs
imobachgs reviewed May 7, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general it looks great. Just a minor comment about the formatList fallback.

Comment thread web/src/agama.ts
mvidner
mvidner approved these changes May 7, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@mvidner mvidner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Consider applying a jq explanation

Comment thread live/src/config.sh
Comment thread web/package/agama-web-ui.spec
install -D -m 0644 --target-directory=%{buildroot}%{_datadir}/agama/web_ui %{_builddir}/agama/dist/*.{css,gz,html,js,json,map,svg}
install -D -m 0644 --target-directory=%{buildroot}%{_datadir}/agama/web_ui/fonts %{_builddir}/agama/dist/fonts/*.ttf
install -D -m 0644 --target-directory=%{buildroot}%{_datadir}/agama/web_ui/assets/logos %{_builddir}/agama/src/assets/products/*.svg
install -D -m 0644 --target-directory=%{buildroot}%{_datadir}/agama/web_ui/assets/logos %{_builddir}/agama/dist/assets/logos/*.svg
Copy link
Copy Markdown
Contributor

@mvidner mvidner May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NP: in the future we might want to go with a web/install.sh approach like the other parts of Agama do (part of #2103). I forgot to do it because I was not doing any changes in the front end at the time

Copy link
Copy Markdown
Contributor Author

@lslezak lslezak May 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, would be nice to unify this to have a single place for changes.

@lslezak @mvidner
Update live/src/config.sh
83a3e05 
Co-authored-by: Martin Vidner <mvidner@suse.com>
@lslezak lslezak merged commit 6375ae0 into master May 7, 2025
9 checks passed
@lslezak lslezak deleted the locale_fixes branch May 7, 2025 08:59
@imobachgs imobachgs mentioned this pull request May 26, 2025
Merged
imobachgs added a commit that referenced this pull request May 26, 2025
@imobachgs
Update to release 15 (#2405)
39bd0ec 
Prepare to release Agama 15:

* #2258
* #2270
* #2277
* #2279
* #2283
* #2284
* #2285
* #2286
* #2287
* #2288
* #2291
* #2292
* #2293
* #2295
* #2297
* #2299
* #2300
* #2301
* #2302
* #2303
* #2305
* #2306
* #2307
* #2308
* #2309
* #2313
* #2314
* #2315
* #2317
* #2318
* #2319
* #2320
* #2321
* #2322
* #2323
* #2324
* #2325
* #2328
* #2329
* #2330
* #2331
* #2335
* #2336
* #2337
* #2338
* #2339
* #2340
* #2342
* #2345
* #2346
* #2348
* #2349
* #2350
* #2351
* #2352
* #2353
* #2354
* #2355
* #2357
* #2358
* #2359
* #2360
* #2361
* #2362
* #2363
* #2364
* #2365
* #2366
* #2368
* #2369
* #2370
* #2371
* #2372
* #2374
* #2377
* #2378
* #2379
* #2380
* #2381
* #2382
* #2384
* #2385
* #2386
* #2388
* #2389
* #2390
* #2391
* #2392
* #2394
* #2397
* #2398
* #2401
* #2403
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@imobachgs imobachgs imobachgs approved these changes

@dgdavid dgdavid dgdavid left review comments

@mvidner mvidner mvidner approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@lslezak @imobachgs @mvidner @dgdavid @yast-bot